dapp staking v3 - part 4 #1053

Dinonard · 2023-10-12T06:19:44Z

Pull Request Summary

Introduces all the remaining dApp staking v3 functionality required for MVP.
Code is almost feature complete, with various TODOs scattered around.

Overview

The following list is an overview of what was introduced & changed in this PR:

genesis configuration
storage for keeping track of era rewards, period end info, tier params & config, dApp rewards
on_initialize logic is implemented (although it might be refactored later)
- handles era changes, period changes, triggering tier reconfiguration and reward calculation
full unlock (exit from dApp staking) will force user to cleanup all storage
introduced tracking of number of contract stake entries per account to limit storage growth per account
introduced calls for claiming rewards: claim_staker_rewards, claim_bonus_reward and claim_dapp_reward
introduced unstake_from_unregistered call for handling specific situation when contract has been unregistered while staked
introduced helper extrinsic call cleanup_expired_entries to help with situations when user has inactive stake entries
- this is important since we have a check in place to prevent growth of contract stake entries per account
- it's possible that some old entries would prohibit user from staking on new contracts
- this situation will happen extremely rarely, but it's necessary to keep
introduced privileged force call to help force eras and periods (primary use is for testing)
added logic for tier configuration calculation, and dApp tier allocation
removed redunant types, refactored lots of existing types

NOTE: Leftover TODOs will be handled in the part 5 PR.

TODO

benchmark tier assignment to assess feasibility of doing it within a single block
simplify ContractStakeAmountSeries in case no off-chain worker is needed (approach from AccountLedger can be reused)

(unsolved TODOs moved to part 5 PR)

PierreOssun

Just had a look at the README part

pallets/dapp-staking-v3/README.md

pallets/dapp-staking-v3/src/types.rs

PierreOssun · 2023-11-07T14:35:16Z

pallets/dapp-staking-v3/src/types.rs

-    /// Old era values cannot be added.
-    OldEra,
+    /// Old or future era values cannot be added.
+    InvalidEra,
    /// Bounded storage capacity exceeded.
    NoCapacity,


What about MaxCapacityExceeded ?

Not sure what you mean - we don't have that error.

Was suggesting to rename the field. But not really needed after all

pallets/dapp-staking-v3/src/types.rs

pallets/dapp-staking-v3/src/lib.rs

PierreOssun · 2023-11-08T14:08:38Z

pallets/dapp-staking-v3/src/lib.rs

+            // Check if the rewards have expired
+            let protocol_state = ActiveProtocolState::<T>::get();
+            ensure!(
+                staked_period >= Self::oldest_claimable_period(protocol_state.period_number()),


If staked period is < of oldest claimable period no fund can be claimed as this ensure! will prevent from it.
In my undestanding staker can still claim the rewards that are between oldest claimable period and now

That's correct. The check does that though, right? Or am I seeing it wrong? 😅

Had a second look and I got it. Actually .staked_period() return the last period eligible for rewards.
I was afraid that, if an user stake for let's say 1 year without claiming, even his from oldest claimable periodtonow` staking era will now be claimable. But they actually will

pallets/dapp-staking-v3/src/lib.rs

pallets/dapp-staking-v3/README.md

pallets/dapp-staking-v3/src/lib.rs

shaunxw · 2023-11-10T14:23:28Z

pallets/dapp-staking-v3/src/lib.rs

+                let staker_reward = Perbill::from_rational(amount, era_reward.staked)
+                    * era_reward.staker_reward_pool;


Suggested change

let staker_reward = Perbill::from_rational(amount, era_reward.staked)

* era_reward.staker_reward_pool;

let staker_reward = Perbill::from_rational(amount, era_reward.staked).checked_mul(era_reward.staker_reward_pool).expect("CheckedMul never fail for PerThings; qed");

I wanted to do this, but there's no checked_mul that takes an integer as an argument, only Self type.

Had to go back to check this, but this is the code covering multiplication with an int:

/// Non-overflow multiplication. /// /// This is tailored to be used with a balance type. impl<N> ops::Mul<N> for $name where N: Clone + UniqueSaturatedInto<$type> + ops::Rem<N, Output=N> + ops::Div<N, Output=N> + ops::Mul<N, Output=N> + ops::Add<N, Output=N> + Unsigned, $type: Into<N>, { type Output = N; fn mul(self, b: N) -> Self::Output { overflow_prune_mul::<N, Self>(b, self.deconstruct(), Rounding::NearestPrefDown) } }

shaunxw · 2023-11-10T14:25:55Z

pallets/dapp-staking-v3/src/lib.rs

+            let eligible_amount = staker_info.staked_amount(Subperiod::Voting);
+            let bonus_reward =
+                Perbill::from_rational(eligible_amount, period_end_info.total_vp_stake)
+                    * period_end_info.bonus_reward_pool;


Same suggestion for explicit checked mul.

shaunxw · 2023-11-10T14:31:08Z

pallets/dapp-staking-v3/src/lib.rs

+            #[pallet::compact] era: EraNumber,
+        ) -> DispatchResult {
+            Self::ensure_pallet_enabled()?;
+            let _ = ensure_signed(origin)?;


Shall we make sure only dApp owner or beneficiary can trigger the claim?

Let me add a TODO for this - there's an open question whether we should allow others to claim rewards in general. We can handle it when considering that question.

shaunxw · 2023-11-10T14:39:27Z

pallets/dapp-staking-v3/src/lib.rs

+            }
+
+            // Account exists since it has locked funds.
+            T::Currency::deposit_into_existing(&account, reward_sum)


Token minting is very security-sensitive. IMO we need some level of sanity check for all minting, to make sure the reward amount never goes terribly wrong. No need to add in this PR but good to add a TODO item.

Agree, and would like this to be done within Tokenomics 2.0 pallet.

I'll work on the design & implementation very soon.

shaunxw · 2023-11-10T14:50:29Z

pallets/dapp-staking-v3/src/lib.rs

+            Ok(())
+        }
+
+        /// Used to force a change of era or subperiod.


Can you add more info in docstring about when this is needed and why?

github-actions · 2023-11-13T08:00:28Z

Package	Line Rate	Branch Rate	Health
chain-extensions/types/assets/src	0%	0%	❌
pallets/pallet-xcm/src	53%	0%	➖
pallets/xvm/src	45%	0%	❌
pallets/dapp-staking-v3/src	74%	0%	➖
chain-extensions/pallet-assets/src	0%	0%	❌
primitives/src/xcm	66%	0%	➖
pallets/dapps-staking/src	81%	0%	✔
chain-extensions/types/xvm/src	0%	0%	❌
precompiles/xcm/src	84%	0%	✔
chain-extensions/dapps-staking/src	0%	0%	❌
pallets/block-reward/src	85%	0%	✔
pallets/custom-signatures/src	51%	0%	➖
precompiles/utils/macro/src	0%	0%	❌
precompiles/utils/src/testing	62%	0%	➖
chain-extensions/types/dapps-staking/src	0%	0%	❌
pallets/ethereum-checked/src	48%	0%	❌
pallets/collator-selection/src	69%	0%	➖
precompiles/substrate-ecdsa/src	78%	0%	➖
precompiles/dapps-staking/src	93%	0%	✔
pallets/dapp-staking-v3/src/test	0%	0%	❌
chain-extensions/xvm/src	0%	0%	❌
precompiles/batch/src	80%	0%	✔
pallets/dapps-staking/src/pallet	85%	0%	✔
primitives/src	65%	0%	➖
pallets/xc-asset-config/src	53%	0%	➖
precompiles/utils/src	68%	0%	➖
precompiles/assets-erc20/src	76%	0%	➖
precompiles/xvm/src	80%	0%	✔
precompiles/sr25519/src	79%	0%	➖
pallets/contracts-migration/src	0%	0%	❌
Summary	61% (3387 / 5584)	0% (0 / 0)	➖

Minimum allowed line rate is 50%

PierreOssun · 2023-11-13T11:03:07Z

pallets/dapp-staking-v3/src/lib.rs

+            // Check if the rewards have expired
+            let protocol_state = ActiveProtocolState::<T>::get();
+            ensure!(
+                staked_period >= Self::oldest_claimable_period(protocol_state.period_number()),


Had a second look and I got it. Actually .staked_period() return the last period eligible for rewards.
I was afraid that, if an user stake for let's say 1 year without claiming, even his from oldest claimable periodtonow` staking era will now be claimable. But they actually will

* dApp Staking v3 Part1 * Feat/dapp staking v3 phase2 (#991) * Phase2 progress * Adapt for 0.9.43 * Abstract sparse vector type * Further modifications * claim unlocked functionality WIP * claim unlocked tested * Relock unlocking * Check era when adding chunk * Custom error for some types * Additional type tests - WIP * More type tests * Review comments * Additional changes * dApp staking v3 - part 3 (#1036) * dApp staking v3 - PR3 * SingularStakingInfo * SingularStakingInfo tests * Stake work * Stake & lots of tests * TODOs, renaming, improvements * Refactoring & cleanup * More fixes * Rework series * Series tests * Minor adjustment * stake test utils & first test * Stake test * stake extrinsic tests * Era & Period change logic * Update era/period transition in mock & tests * on_init tests & refactoring * Unstake & some minor improvements * Lots of type tests for unstake * More tests * More types test * Testing utils for unstake, some TODOs * Unstake tests * Minor adjustments * Fixes * Additional scenario * Address review comments * Correct unstake amount * Tests * dapp staking v3 - part 4 (#1053) * EraRewardSpan * Initial version of claim_staker_reward * Tests * Test utils for claim-staker * Bug fixes, improvements * Claim improvements & some tests * Refactoring in progress * Refactoring continued * Refactoring progress * Refactoring finished * Bonus rewards * Docs & some minor changes * Comments, tests, improved coverage * Tier params & config init solution * Tier reward calculation WIP * Tier assignemnt * Minor cleanup * Claim dapp rewards * Claim dapp reward tests * unstake from unregistered call * Extra traits * fixes * Extra calls * Refactoring * More refactoring, improvements, TODO solving * Local integration * Genesis config * Add forcing call * try runtime build fix * Minor changes * Minor * Formatting * Benchmarks INIT * Compiling benchmarks * Fix * dapp tier calculation benchmark * Measured tier assignment * Decending rewards in benchmarks * Series refactoring & partial tests * Comments, minor changes * Tests, improvements * More tests, some minor refactoring * Formatting * More benchmarks & experiments, refactoring * Readme, docs * Minor renaming, docs * More docs * More docs * Minor addition * Review comment fixes & changes * Minor change * Review comments * Update frontier to make CI pass * Fix for cleanup * dapp staking v3 - part 5 (#1087) * EraRewardSpan * Initial version of claim_staker_reward * Tests * Test utils for claim-staker * Bug fixes, improvements * Claim improvements & some tests * Refactoring in progress * Refactoring continued * Refactoring progress * Refactoring finished * Bonus rewards * Docs & some minor changes * Comments, tests, improved coverage * Tier params & config init solution * Tier reward calculation WIP * Tier assignemnt * Minor cleanup * Claim dapp rewards * Claim dapp reward tests * unstake from unregistered call * Extra traits * fixes * Extra calls * Refactoring * More refactoring, improvements, TODO solving * Local integration * Genesis config * Add forcing call * try runtime build fix * Minor changes * Minor * Formatting * Benchmarks INIT * Compiling benchmarks * Fix * dapp tier calculation benchmark * Measured tier assignment * Decending rewards in benchmarks * Series refactoring & partial tests * Comments, minor changes * Tests, improvements * More tests, some minor refactoring * Formatting * More benchmarks & experiments, refactoring * Readme, docs * Minor renaming, docs * More docs * More docs * Minor addition * Review comment fixes & changes * Minor change * Review comments * Update frontier to make CI pass * dApp staking v3 - part 5 * Make check work * Limit map size to improve benchmarks * Fix for cleanup * Minor fixes, more tests * More fixes & test * Minor changes * More tests * More tests, some clippy fixes * Finished with type tests for now * Log, remove some TODOs * Changes * Bug fix for unstake era, more tests * Formatting, extra test * Unregistered unstake, expired cleanup, test utils & tests * Cleanup tests, minor fixes * force tests * Remove TODOs * Tier assignment test WIP * Finish dapp tier assignment test, fix reward calculation bug * Some renaming, test utils for era change WIP * Fix minor issues, propagaste renaming * More checks * Finish on_init tests * Comments, docs, some benchmarks * Benchmark progress * More benchmarks * Even more benchmarks * All extrinsics benchmarked * Expand tests * Comment * Missed error test * Review comments * Pallet inflation (#1091) * Pallet inflation * Hooks * Functionality, mock & benchmarks * Empty commit since it seems last push didn't work properly * Tests, fixes * More tests, minor fixes&changes * Zero divison protection, frontier update * More tests, minor changes * Integration & renaming * Genesis integration, more tests * Final integration * Cleanup deps * Division with zero test * Comments * More minor changes * Improve test coverage * Integrate into pallet-timestamp * Remove timestamp * Fix * review comments * toml format * dApp Staking v3 - part 6 (#1093) * dApp staking v3 part 6 * Minor refactor of benchmarks * Weights integration * Fix * remove orig file * Minor refactoring, more benchmark code * Extract on_init logic * Some renaming * More benchmarks * Full benchmarks integration * Testing primitives * staking primitives * dev fix * Integration part1 * Integration part2 * Reward payout integration * Replace lock functionality with freeze * Cleanup TODOs * More negative tests * Frozen balance test * Zero div * Docs for inflation * Rename is_active & add some more docs * More docs * pallet docs * text * scripts * More tests * Test, docs * Review comment * dApp staking v3 - part 7 (#1099) * dApp staking v3 part 6 * Minor refactor of benchmarks * Weights integration * Fix * remove orig file * Minor refactoring, more benchmark code * Extract on_init logic * Some renaming * More benchmarks * Full benchmarks integration * Testing primitives * staking primitives * dev fix * Integration part1 * Integration part2 * Reward payout integration * Replace lock functionality with freeze * Cleanup TODOs * More negative tests * Frozen balance test * Zero div * Docs for inflation * Rename is_active & add some more docs * More docs * pallet docs * text * scripts * More tests * Test, docs * Review comment * Runtime API * Changes * Change dep * Comment * Formatting * Expired entry cleanup * Cleanup logic test * Remove tier labels * fix * Improve README * Improved cleanup * Review comments * Docs * Formatting * Typo * Fix - bonus reward update ledger (#1102) * Bonus claim reduced contract_stake_count * Extra test * dApp Staking Migration (#1101) * dApp Staking Migration * Events, benchmark prep * benchmarks * Benchmarks & mock * weights * limit * Use extrinsic call * Solved todos, docs, improvements * Maintenance mode, on_runtime_upgrade logic * Tests, refactoring * Finish * More tests * Type cleanup * try-runtime checks * deps * Improve docs * Fixes & try-runtime testing modifications * Fix test * repeated * Minor improvements * Improvements * taplo * Minor rename * Feat/dapp staking v3 precompiles (#1096) * Init commit * All legacy calls covered * v2 interface first definition * Rename * Resolve merge errors * TODO * v2 init implementation * Prepared mock * todo * Migration to v2 utils * More adjustments * Finish adapting implementation to v2 * Mock & fixes * Primitive smart contract * Fix dsv3 test * Prepare impl & mock * Remove redundant code, adjust mock & prepare tests * Tests & utils * Test for legacy getters/view functions * More legacy tests * v1 interface covered with tests * Minor refactor, organization, improvements * v2 tests * Cleanup TODOs * More tests * Updates * docs * Fixes * Address review comments * Adjustments * Audit comments * Fix mock * FMT * Review comments * Fix for incorrect freeze amount (#1111) * dApp Staking v3 - Shibuya integration (#1109) * dApp staking v3 - Shibuya integration * Init * Fix build * Fix find/replace doc mess * Migration & disable * More integration * Progress * Adjusted integration * Finished integration * Additional modifications & cleanup * Move comment * Fixes * Shibuya integration tests fix & proxy * Renaming * Integration test fixes & legacy support * Adjust for benchmarks * Remove chain-extension, small updates * fixes * Partial weights * Minor changes * Benchmark fixes * dApp staking weights * Weights, deps * Remove redundant storage item * Inflation params, resolve TODOs * Optimize lengthy benchmark * Integration test * Sort out more TODOs * Benchmark optimization * Fix seed * Remove spec version bump * Fix integration test * Weights update

EraRewardSpan

0a7c519

Dinonard force-pushed the feat/dapp-staking-v3-phase4 branch from 9c316e2 to 0a7c519 Compare October 12, 2023 06:21

Dinonard added 2 commits October 12, 2023 18:27

Initial version of claim_staker_reward

e598d1b

Tests

fff0656

Dinonard added the runtime This PR/Issue is related to the topic “runtime”. label Oct 12, 2023

Dinonard added 25 commits October 13, 2023 11:42

Test utils for claim-staker

0daae3a

Bug fixes, improvements

ebae663

Claim improvements & some tests

c76ade9

Refactoring in progress

096726e

Refactoring continued

39c6cf0

Refactoring progress

e77327d

Refactoring finished

025f0b7

Bonus rewards

80a0e4f

Docs & some minor changes

82c71ad

Comments, tests, improved coverage

582aef4

Tier params & config init solution

b25ccb1

Tier reward calculation WIP

3d2d146

Tier assignemnt

894d10d

Minor cleanup

ff61cf1

Claim dapp rewards

3ded046

Claim dapp reward tests

f2f8a5f

unstake from unregistered call

03d88f4

Extra traits

ea3e23d

fixes

7558682

Extra calls

e0f19e1

Refactoring

4926838

More refactoring, improvements, TODO solving

d47bcf4

Local integration

0197c95

Genesis config

4ea8556

Add forcing call

589575b

Dinonard added 5 commits October 31, 2023 17:17

Comments, minor changes

c586a6c

Tests, improvements

5cf60a1

More tests, some minor refactoring

7a3c634

Formatting

c47d36e

More benchmarks & experiments, refactoring

d36839c

Dinonard force-pushed the feat/dapp-staking-v3-phase4 branch from bf15fc9 to d36839c Compare November 2, 2023 16:05

Dinonard added 5 commits November 2, 2023 18:28

Readme, docs

d0d196e

Minor renaming, docs

9723d9f

More docs

24b4b53

More docs

408eaad

Minor addition

fd65f20

Dinonard marked this pull request as ready for review November 7, 2023 13:44

Dinonard requested review from shaunxw, ashutoshvarma and PierreOssun November 7, 2023 13:45

PierreOssun reviewed Nov 7, 2023

View reviewed changes

pallets/dapp-staking-v3/README.md Show resolved Hide resolved

pallets/dapp-staking-v3/README.md Show resolved Hide resolved

PierreOssun reviewed Nov 7, 2023

View reviewed changes

PierreOssun reviewed Nov 8, 2023

View reviewed changes

pallets/dapp-staking-v3/src/types.rs Outdated Show resolved Hide resolved

pallets/dapp-staking-v3/src/types.rs Show resolved Hide resolved

PierreOssun reviewed Nov 8, 2023

View reviewed changes

Dinonard added 2 commits November 9, 2023 17:10

Review comment fixes & changes

8fe1563

Minor change

3b894ef

shaunxw reviewed Nov 10, 2023

View reviewed changes

Dinonard added 2 commits November 10, 2023 17:00

Review comments

7cc82da

Update frontier to make CI pass

cc1efcc

PierreOssun approved these changes Nov 13, 2023

View reviewed changes

Dinonard changed the title ~~dapp staking v3 - part4~~ dapp staking v3 - part 4 Nov 13, 2023

Dinonard merged commit 9b5f988 into feat/dapp-staking-v3 Nov 14, 2023
13 checks passed

Dinonard deleted the feat/dapp-staking-v3-phase4 branch November 14, 2023 06:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dapp staking v3 - part 4 #1053

dapp staking v3 - part 4 #1053

Dinonard commented Oct 12, 2023 •

edited

Loading

PierreOssun left a comment

PierreOssun Nov 7, 2023

Dinonard Nov 9, 2023

PierreOssun Nov 13, 2023

PierreOssun Nov 8, 2023

Dinonard Nov 9, 2023

PierreOssun Nov 13, 2023

shaunxw Nov 10, 2023

Dinonard Nov 10, 2023

shaunxw Nov 10, 2023

shaunxw Nov 10, 2023

Dinonard Nov 10, 2023

shaunxw Nov 10, 2023

Dinonard Nov 10, 2023

shaunxw Nov 10, 2023

Dinonard Nov 10, 2023

github-actions bot commented Nov 13, 2023

PierreOssun Nov 13, 2023

		let staker_reward = Perbill::from_rational(amount, era_reward.staked)
		* era_reward.staker_reward_pool;

dapp staking v3 - part 4 #1053

dapp staking v3 - part 4 #1053

Conversation

Dinonard commented Oct 12, 2023 • edited Loading

Pull Request Summary

Overview

TODO

PierreOssun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 13, 2023

Choose a reason for hiding this comment

Dinonard commented Oct 12, 2023 •

edited

Loading